-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: GroupedListV2 scrollToIndex now works correctly #29332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: GroupedListV2 scrollToIndex now works correctly #29332
Conversation
GroupedListV2 flattens the list of items to better support virtualization. This means that we need to make the `index` passed into `scrollToIndex()` to the flattened list. Additionally, to properly measure heights for virtualization we need to be able to account for group headers and footers so APIs have been updated to provide this information to users. Finally, a new example has been added to demonstrate how to use this feature.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4980650:
|
🕵 fluentuiv8 No visual regressions between this PR and main |
📊 Bundle size report🤖 This report was generated against e0d871e6236b9ef86b8566daf7c8ba61a7325b1f |
Asset size changes
Baseline commit: e0d871e6236b9ef86b8566daf7c8ba61a7325b1f (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 620 | 619 | 5000 | |
| Breadcrumb | mount | 1689 | 1687 | 1000 | |
| Checkbox | mount | 1645 | 1641 | 5000 | |
| CheckboxBase | mount | 1482 | 1470 | 5000 | |
| ChoiceGroup | mount | 2917 | 2906 | 5000 | |
| ComboBox | mount | 625 | 644 | 1000 | |
| CommandBar | mount | 6167 | 6178 | 1000 | |
| ContextualMenu | mount | 11548 | 11723 | 1000 | |
| DefaultButton | mount | 725 | 728 | 5000 | |
| DetailsRow | mount | 2157 | 2175 | 5000 | |
| DetailsRowFast | mount | 2137 | 2130 | 5000 | |
| DetailsRowNoStyles | mount | 2025 | 1961 | 5000 | |
| Dialog | mount | 2587 | 2631 | 1000 | |
| DocumentCardTitle | mount | 226 | 234 | 1000 | |
| Dropdown | mount | 1965 | 1999 | 5000 | |
| FocusTrapZone | mount | 1086 | 1137 | 5000 | |
| FocusZone | mount | 1056 | 1048 | 5000 | |
| GroupedList | mount | 40392 | 40838 | 2 | |
| GroupedList | virtual-rerender | 17293 | 19545 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 50869 | 49883 | 2 | |
| GroupedListV2 | mount | 220 | 222 | 2 | |
| GroupedListV2 | virtual-rerender | 216 | 211 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 224 | 237 | 2 | |
| IconButton | mount | 1106 | 1073 | 5000 | |
| Label | mount | 335 | 337 | 5000 | |
| Layer | mount | 2754 | 2755 | 5000 | |
| Link | mount | 393 | 396 | 5000 | |
| MenuButton | mount | 936 | 943 | 5000 | |
| MessageBar | mount | 21558 | 21755 | 5000 | |
| Nav | mount | 1921 | 1950 | 1000 | |
| OverflowSet | mount | 797 | 777 | 5000 | |
| Panel | mount | 1777 | 1821 | 1000 | |
| Persona | mount | 738 | 704 | 1000 | |
| Pivot | mount | 839 | 859 | 1000 | |
| PrimaryButton | mount | 844 | 848 | 5000 | |
| Rating | mount | 4585 | 4603 | 5000 | |
| SearchBox | mount | 884 | 909 | 5000 | |
| Shimmer | mount | 1899 | 1862 | 5000 | |
| Slider | mount | 1320 | 1318 | 5000 | |
| SpinButton | mount | 2803 | 2872 | 5000 | |
| Spinner | mount | 381 | 399 | 5000 | |
| SplitButton | mount | 1841 | 1819 | 5000 | |
| Stack | mount | 397 | 410 | 5000 | |
| StackWithIntrinsicChildren | mount | 863 | 853 | 5000 | |
| StackWithTextChildren | mount | 2612 | 2578 | 5000 | |
| SwatchColorPicker | mount | 6132 | 6041 | 5000 | |
| TagPicker | mount | 1472 | 1461 | 5000 | |
| Text | mount | 379 | 370 | 5000 | |
| TextField | mount | 937 | 938 | 5000 | |
| ThemeProvider | mount | 846 | 842 | 5000 | |
| ThemeProvider | virtual-rerender | 590 | 600 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1289 | 1315 | 5000 | |
| Toggle | mount | 602 | 606 | 5000 | |
| buttonNative | mount | 184 | 186 | 5000 |
sopranopillow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the two comments, looks great!
packages/react-examples/src/react/DetailsList/DetailsList.GroupedV2.ScrollToIndex.Example.tsx
Outdated
Show resolved
Hide resolved
packages/react/src/components/GroupedList/GroupedListV2.types.ts
Outdated
Show resolved
Hide resolved
packages/react/src/components/GroupedList/GroupedListV2.types.ts
Outdated
Show resolved
Hide resolved
packages/react/src/components/GroupedList/GroupedListV2.base.tsx
Outdated
Show resolved
Hide resolved
packages/react/src/components/GroupedList/GroupedListV2.base.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Esteban Munoz Facusse <[email protected]> Co-authored-by: Makoto Morimoto <[email protected]>
* master: (35 commits) bugfix(react-dialog): removes context hooks invocations from styles hook (microsoft#29396) chore(react-dialog): exports DialogSurface context types and hooks (microsoft#29397) Minimum height feature for bar chart (microsoft#29359) (feat) Breadcrumb - register items via context (microsoft#29393) fix: Consider all parents as scroll parents (microsoft#29378) Changes to enable charting on fluent docsite (microsoft#29210) bugfix: ensure TreeItem emits events properly (microsoft#29390) chore(deps): bump get-func-name from 2.0.0 to 2.0.2 (microsoft#29320) chore: migrate from getNativeElementProps to getIntrinsicElementProps (microsoft#29387) applying package updates applying package updates fix: Icon in disabled Button shouldn't change color on hover or pressed (microsoft#29342) Squish changes for rebase (microsoft#28705) fix: GroupedListV2 scrollToIndex now works correctly (microsoft#29332) Fix 27482: ProgressBar has an optional state variable (microsoft#29366) remove a11y test ux from theme designer (microsoft#29379) feat: preview release (microsoft#29377) Fixed divider in the tooltip (microsoft#29357) fix (microsoft#29376) docs: Initial documentation pass (microsoft#29372) ...
Previous Behavior
GroupedListV2would not scroll to the correct index when usingscrollToIndex().New Behavior
GroupedListV2 flattens the list of items to better support virtualization. This means that we need to map the
indexpassed intoscrollToIndex()to the flattened list.Additionally, to properly measure heights for virtualization we need to be able to account for group headers and footers so APIs have been updated to provide this information to users.
Finally, a new example has been added to demonstrate how to use this feature.
Related Issue(s)
scrollToIndex()#29169